Refactor information.py with lots of changes that makes the overall code cleaner and faster#741
Conversation
…efault, and non-verified users cannot use this command.
…end it to a list, and then send them it at once (less spammy)
…ping for just 1 or 2 uses (e.g. Union, and Message).
…the second cannot be true making it unneccesary to check
Denayder
requested review from
kosayoda and
tagptroll1
and removed request for
a team
Denayder
requested review from
sco1
and removed request for
kosayoda and
tagptroll1
ikuyarihS
added
a: information
ikuyarihS
requested review from
a team and
fiskenslakt
and removed request for
a team
MarkKoz
added
s: waiting for author
… the __class__ attribute, and re-add the None check for !user roles.
|
All the requested changes should have been made, although, I can't seem to figure out why the test is failing, even though I did update it to reflect the changes made. Help on this would be appreciated, since I have zero clue 😓 |
It's due to |
|
To expand on the above, this is what comes out of the Which obviously doesn't match what we have in the unit test, though Discord renders it fine because they (usually) strip the leading whitespace. One approach that can be used is to insert the channel list after # Because channel_counts lacks leading whitespace, it breaks the dedent if it's inserted directly by the
# f-string. While this is correctly formated by Discord, it makes unit testing difficult. To keep the formatting
# without joining a tuple of strings we can use a Template string to insert the already-formatted channel_counts
# after the dedent is made.
embed_description = Template(
textwrap.dedent(f"""
**Server information**
Created: {created}
Voice region: {region}
Features: {features}
**Counts**
Members: {member_count:,}
Roles: {roles}
$channel_counts
**Members**
{constants.Emojis.status_online} {statuses[Status.online]:,}
{constants.Emojis.status_idle} {statuses[Status.idle]:,}
{constants.Emojis.status_dnd} {statuses[Status.dnd]:,}
{constants.Emojis.status_offline} {statuses[Status.offline]:,}
""")
).substitute({"channel_counts": channel_counts})
embed = Embed(colour=Colour.blurple(), description=embed_description)Alternatively, you could also use string concatenation, with our without a join: embed = Embed(
colour=Colour.blurple(),
description=(
"**Server information**\n"
f"Created: {created}\n"
f"Voice region: {region}\n"
f"Features: {features}\n\n"
"**Counts**\n"
f"Members: {member_count:,}\n"
f"Roles: {roles}\n"
f"{channel_counts}\n\n"
"**Members**\n"
f"{constants.Emojis.status_online} {statuses[Status.online]:,}\n"
f"{constants.Emojis.status_idle} {statuses[Status.idle]:,}\n"
f"{constants.Emojis.status_dnd} {statuses[Status.dnd]:,}\n"
f"{constants.Emojis.status_offline} {statuses[Status.offline]:,}\n"
)
)embed = Embed(
colour=Colour.blurple(),
description="\n".join(
(
"**Server information**",
f"Created: {created}",
f"Voice region: {region}",
f"Features: {features}",
"\n**Counts**",
f"Members: {member_count:,}",
f"Roles: {roles}",
f"{channel_counts}",
"\n**Members**",
f"{constants.Emojis.status_online} {statuses[Status.online]:,}",
f"{constants.Emojis.status_idle} {statuses[Status.idle]:,}",
f"{constants.Emojis.status_dnd} {statuses[Status.dnd]:,}",
f"{constants.Emojis.status_offline} {statuses[Status.offline]:,}",
)
)
)The first approach probably makes the most sense. |
|
Apologies for the delay, and thanks for the assistance @sco1. All changes have been made and checks have been fixed. This should be ready for another review 😁 |
MarkKoz
added
status: needs review
and removed
s: waiting for author
4 participants
Hello. I created #740 yesterday, but with lack of knowledge of forking and proper commit messages, the commit history became a slight bit messy. I'm here again with pretty much the same PR, with a few changes here and there. Thanks to @sco1 for the helpful advice so far.
A change log can be found here and in the commit messages:
@everyoneor not. Simply skipping the first element in the.roleswill skip the@everyonerole.Counter(), and uses the__class__attribute for channel counting.!role. Instead, append a list with all the failed roles, and send 1 message. This should be less spammy.discord&typingwas imported, and then the entire module was imported.ifstatement to anelifstatement.Some concern was brought up regarding the use of the
__class__attribute- I'm not sure if the use ofCounter()fixes this concern or not.Thank you once again for giving this PR a look. Let me know if there is anything you would like changed. :)